[ThienDuc3112] iP by ThienDuc3112 · Pull Request #100 · nus-cs2113-AY2425S1/ip

ThienDuc3112 · 2024-09-03T06:09:58Z

No description provided.

Last commit I forgot to add list command

Also error handling

KHsienKit · 2024-09-04T04:42:20Z

@@ -0,0 +1,111 @@
+import java.util.*;


Avoid using wildcard imports
Import only the functions that you need

KHsienKit · 2024-09-04T04:44:25Z

+        isOn = true;
+        datas = new ArrayList<>();
+
+        String greeting = "____________________________________________________________\n" +


use "final" and SCREAMING_SNAKE_CASE for the greeting variable as it is a constant

KHsienKit · 2024-09-04T04:48:06Z

+        return isDone;
+    }
+
+    public void mark(){


add a space before the "{" to keep layout consistent

KHsienKit · 2024-09-04T04:49:24Z

+            String line = in.nextLine().strip();
+            System.out.println("____________________________________________________________");
+
+            if (line.equals("bye")) {


Followed if else layout convention. Good job

seanngja

LGTM!

seanngja · 2024-09-04T05:43:45Z

@@ -0,0 +1,111 @@
+import java.util.*;


Perhaps you could import just the libraries you need?

seanngja · 2024-09-04T05:45:13Z

+            } else if (line.equals("list")) {
+                list();
+            } else if (line.startsWith("mark ")) {
+                mark(line);
+            } else if (line.startsWith("unmark ")) {
+                unmark(line);
+            } else if (line.startsWith("todo ")) {
+                todo(line);
+            } else if (line.startsWith("event ")) {
+                event(line);
+            } else if (line.startsWith("deadline ")) {
+                deadline(line);
+            } else {
+                add(line);
+            }


Perhaps you could refactor this into another function? This chunk is long and hard to read.

seanngja · 2024-09-04T05:46:15Z

+
+    private static void todo(String line) {
+        String todo = line.substring(line.indexOf(" ") + 1).trim();
+        Todo task =new Todo(todo);


Fix the formatting after the = sign.

seanngja · 2024-09-04T05:48:19Z

+
+    public static void main(String[] args) {
+        isOn = true;
+        datas = new ArrayList<>();


Perhaps you could use a more descriptive name for your array list, as it is unclear what is being stored here.

seanngja · 2024-09-04T05:51:52Z

+        return "[" +
+                (isDone ? "X" : " ") +
+                "] " +
+                task;


Perhaps you could write this all in 1 line of code to make your code more readable.

Mahesh1772

Solid effort

Code is following most of the code quality standards. The naming would need to be improved for better scalability so that fellow developers can understand the use of the method and variable easily. Consider adding JavaDocs in future commits

Mahesh1772 · 2024-09-20T08:51:41Z

+    private static List<Task> taskList;
+
+    public static void main(String[] args) {
+        isOn = true;


Consider using more descriptive names, that is easily identified to serve a definite purpose

Mahesh1772 · 2024-09-20T08:53:40Z

+                isOn = false;
+            } else if (line.equals("list")) {
+                list();
+            } else if (line.startsWith("mark ")) {
+                mark(line);
+            } else if (line.startsWith("unmark ")) {
+                unmark(line);
+            } else if (line.startsWith("todo ")) {
+                todo(line);
+            } else if (line.startsWith("event ")) {
+                event(line);
+            } else if (line.startsWith("deadline ")) {
+                deadline(line);
+            } else {
+                System.out.println("Unknown command: " + line);
+            }


Consider using switch case for this case, as the different cases of input is pre-determined

Mahesh1772 · 2024-09-20T08:54:27Z

+        }
+    }
+
+    private static void list() {


Consider naming it as listTasks to avoid confusion with the inbuilt type

Mahesh1772 · 2024-09-20T08:55:21Z

+            int option = Integer.parseInt(line.substring(line.indexOf(" ") + 1));
+            System.out.println("Nice! I've marked this task as done:");
+            taskList.get(option - 1).mark();
+            System.out.println(taskList.get(option - 1).toString());
+        } catch (Exception e) {
+            System.out.println("Invalid input, input must be a positive integer and must exist");
+        }
+    }
+
+    private static void unmark(String line) {
+        try {
+            int option = Integer.parseInt(line.substring(line.indexOf(" ") + 1));
+            System.out.println("OK, I've marked this task as not done yet:");
+            taskList.get(option - 1).unmark();
+            System.out.println(taskList.get(option - 1).toString());
+        } catch (Exception e) {
+            System.out.println("Invalid input, input must be a positive integer and must exist");
+        }


Consider using the same method or a central part for both functions as to reduce the number of duplicate lines of code

Mahesh1772 · 2024-09-20T08:56:47Z

+        System.out.println("Now you have " + taskList.size() + " tasks in the list.");
+    }
+
+    private static void event(String line) {


Consider naming the methods better for easy maintainability and scalability

Mahesh1772 · 2024-09-20T08:57:17Z

+    String from;
+    String to;


Consider something like fromDate , toDate

Mahesh1772 · 2024-09-20T08:57:48Z

+        return "[" +
+                (isDone ? "X" : " ") +
+                "] " +
+                task;


Consider same line implementation to increase code readability

Mahesh1772 · 2024-09-20T08:58:20Z

+
+public class Utilities {
+    public static Hashtable<String, String> getCommandArgument(String line) {
+        Hashtable<String, String> args = new Hashtable<>();


Try to not use vague names for variables

Can

ThienDuc3112 added 7 commits August 28, 2024 13:46

First commit

eff6d00

Echo

175416a

Add, List

a464a71

Actually adding list command

e4d98c5

Last commit I forgot to add list command

Mark as Done

db6b1af

Coding standard

c4699cf

Also error handling

Todos, Events and Deadlines

75ab4ec

KHsienKit reviewed Sep 4, 2024

View reviewed changes

seanngja reviewed Sep 4, 2024

View reviewed changes

ThienDuc3112 added 3 commits September 5, 2024 21:29

Improve code quality

b404f55

Add error handling

69c8498

Merge branch 'branch-Level-5'

4287dc0

Mahesh1772 reviewed Sep 20, 2024

View reviewed changes

ThienDuc3112 and others added 14 commits September 20, 2024 23:42

Adding delete

cc1c022

Merge branch 'branch-Level-6'

5660dd7

Save feature

f2e5d2a

Merge branch 'branch-Level-7'

de42978

More OOP

3032c85

Find command

418a21a

Merge branch 'branch-Level-9'

3d6dda7

Adding documentation

c2c4b65

Merge branch 'branch-A-JavaDoc'

7c1bec4

User guide

a4d16bc

Update the outside readme aswell

8589761

Don't allow empty task

1566ce5

Adding custom exception type

515c9ad

Merge pull request #1 from ThienDuc3112/branch-Exception

5c330cb

Can

Conversation

ThienDuc3112 commented Sep 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanngja left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mahesh1772 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants